ART: Fix invalid access and DCHECK in verifier
If we get a throwing failure when setting types from the signature,
the work instruction index is still invalid. Do not try to copy the
line then.
As a throwing failure might happen in the above instance, but the
flow analysis expects to have a cleared failure flag before processing
each instruction, clear the flag.
Bug: 21645819
Bug: 22080519
Change-Id: I224c4dad98fa5bb50e62210f0ee30c0dd020e3a6
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 1f7dd58..085f741 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -385,7 +385,7 @@
bool allow_thread_suspension)
: self_(self),
reg_types_(can_load_classes),
- work_insn_idx_(-1),
+ work_insn_idx_(DexFile::kDexNoIndex),
dex_method_idx_(dex_method_idx),
mirror_method_(method),
method_access_flags_(method_access_flags),
@@ -409,7 +409,8 @@
has_check_casts_(false),
has_virtual_or_interface_invokes_(false),
verify_to_dump_(verify_to_dump),
- allow_thread_suspension_(allow_thread_suspension) {
+ allow_thread_suspension_(allow_thread_suspension),
+ link_(nullptr) {
self->PushVerifier(this);
DCHECK(class_def != nullptr);
}
@@ -599,12 +600,16 @@
// We need to save the work_line if the instruction wasn't throwing before. Otherwise we'll
// try to merge garbage.
// Note: this assumes that Fail is called before we do any work_line modifications.
- const uint16_t* insns = code_item_->insns_ + work_insn_idx_;
- const Instruction* inst = Instruction::At(insns);
- int opcode_flags = Instruction::FlagsOf(inst->Opcode());
+ // Note: this can fail before we touch any instruction, for the signature of a method. So
+ // add a check.
+ if (work_insn_idx_ < DexFile::kDexNoIndex) {
+ const uint16_t* insns = code_item_->insns_ + work_insn_idx_;
+ const Instruction* inst = Instruction::At(insns);
+ int opcode_flags = Instruction::FlagsOf(inst->Opcode());
- if ((opcode_flags & Instruction::kThrow) == 0 && CurrentInsnFlags()->IsInTry()) {
- saved_line_->CopyFromLine(work_line_.get());
+ if ((opcode_flags & Instruction::kThrow) == 0 && CurrentInsnFlags()->IsInTry()) {
+ saved_line_->CopyFromLine(work_line_.get());
+ }
}
}
break;
@@ -1236,6 +1241,9 @@
PrependToLastFailMessage(prepend);
return false;
}
+ // We may have a runtime failure here, clear.
+ have_pending_runtime_throw_failure_ = false;
+
/* Perform code flow verification. */
if (!CodeFlowVerifyMethod()) {
DCHECK_NE(failures_.size(), 0U);
diff --git a/test/800-smali/expected.txt b/test/800-smali/expected.txt
index 80b4f57..2196a88 100644
--- a/test/800-smali/expected.txt
+++ b/test/800-smali/expected.txt
@@ -22,4 +22,5 @@
b/21863767
b/21886894
b/22080519
+b/21645819
Done!
diff --git a/test/800-smali/smali/b_21645819.smali b/test/800-smali/smali/b_21645819.smali
new file mode 100644
index 0000000..195d662
--- /dev/null
+++ b/test/800-smali/smali/b_21645819.smali
@@ -0,0 +1,9 @@
+.class public LB21645819;
+.super Ljava/lang/Object;
+
+# The method declares a parameter of an inaccessible class. This should not abort/kill us.
+
+.method public static run(Lpkg/ProtectedClass;)V
+.registers 10
+ return-void
+.end method
\ No newline at end of file
diff --git a/test/800-smali/src/Main.java b/test/800-smali/src/Main.java
index 337d0d9..e6f065e 100644
--- a/test/800-smali/src/Main.java
+++ b/test/800-smali/src/Main.java
@@ -91,6 +91,8 @@
null));
testCases.add(new TestCase("b/22080519", "B22080519", "run", null,
new NullPointerException(), null));
+ testCases.add(new TestCase("b/21645819", "B21645819", "run", new Object[] { null },
+ null, null));
}
public void runTests() {
diff --git a/test/800-smali/src/pkg/ProtectedClass.java b/test/800-smali/src/pkg/ProtectedClass.java
new file mode 100644
index 0000000..b262155
--- /dev/null
+++ b/test/800-smali/src/pkg/ProtectedClass.java
@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package pkg;
+
+class ProtectedClass {
+}
